feat: add multi-chain transaction history hooks #1233
Conversation
…e into or-condition-subgraph
…e into or-condition-subgraph
…e into or-condition-subgraph
…rum-token-bridge into or-condition-subgraph
…e into multi-chain-hooks
| @@ -117,6 +117,9 @@ function parseTransferToMergedTransaction( | |||
| tokenAddress: getUsdcTokenAddressFromSourceChainId(sourceChainId), | |||
| depositStatus: DepositStatus.CCTP_DEFAULT_STATE, | |||
| isCctp: true, | |||
| // TODO: Fix those when adding CCTP | |||
There was a problem hiding this comment.
As discussed, this will be added in another PR
| @@ -132,7 +132,7 @@ export default async function handler( | |||
| l1Token, | |||
| tokenAmount, | |||
| isClassic, | |||
| l2BlockTimestamp, | |||
| timestamp: l2BlockTimestamp, | |||
There was a problem hiding this comment.
I would keep this as it is in the subgraph, as it could cause confusion
| l2TxHash: string | ||
| l2BlockNum: string | ||
| parentChainId: number | ||
| chainId: number |
There was a problem hiding this comment.
Let's do childChainId everywhere. Christophe's useNetworksRelationship hook will give us parentChain and childChain
| @@ -84,6 +85,64 @@ export const transformDeposits = ( | |||
| }) | |||
| } | |||
|
|
|||
| export const transformDeposit = (tx: Deposit): MergedTransaction => { | |||
There was a problem hiding this comment.
I think extracting transformDeposit and transformWithdrawal as a separate PR (before this one) might make the diff smaller
Edit: #1310
| export interface Transaction extends TransactionBase { | ||
| export interface Transaction | ||
| extends TransactionBase, | ||
| AdditionalTransferProperties { |
There was a problem hiding this comment.
Can we just add these fields directly to Transaction?
| const finalTransactions: Transaction[] = await Promise.all( | ||
| ethDepositsFromSubgraph.map(depositTx => | ||
| updateAdditionalDepositData(depositTx, l1Provider, l2Provider) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Let's move this out of fetchDeposits and into useDeposits in a separate PR. That way we could reuse the current fetchDeposits for the new transaction history and reduce the diff. Same thing for withdrawals.
| return tx.direction === 'deposit' | ||
| } | ||
|
|
||
| async function transformTransaction(tx: DepositOrWithdrawal) { |
There was a problem hiding this comment.
Let's add a return type to be safe it's always good
| const [page, setPage] = useState(0) | ||
| const [loading, setLoading] = useState(true) | ||
|
|
||
| const { address } = useAccount() |
There was a problem hiding this comment.
Make this a prop for the hook. Reasoning: we want to keep this independent of the connected wallet, e.g. if we ever wanna have a separate transaction history page that you can provide the address for through a query param
| [page, transactions] | ||
| ) | ||
|
|
||
| const { data, error } = useSWRImmutable( |
There was a problem hiding this comment.
Maybe there's a way to replace this with https://swr.vercel.app/docs/pagination.en-US#infinite-loading
| * Fetches transaction history only, without mapping additional info that would require a lot of RPC calls. | ||
| * The data is collected for multiple chain pairs, and sorted by date. | ||
| */ | ||
| const useMultiChainTransactionList = () => { |
There was a problem hiding this comment.
I feel like this should belong to the useTransactionListByDirection so it gives us merged deposits & withdrawals, sorted and everything. We can remove the direction prop.
There was a problem hiding this comment.
Then we end up with 2 hooks instead of 3.
| * Fetches transaction history only, and only for a specific direction. | ||
| * The query could be e.g. deposits or withdrawals. | ||
| */ | ||
| const useTransactionListByDirection = ( |
There was a problem hiding this comment.
| const useTransactionListByDirection = ( | |
| const useTransactionHistoryWithoutStatuses = ( |
| * Maps additional info to previously fetches transaction history, starting with the earliest data. | ||
| * This is done in small batches to safely meet RPC limits. | ||
| */ | ||
| export const useCompleteMultiChainTransactions = () => { |
There was a problem hiding this comment.
| export const useCompleteMultiChainTransactions = () => { | |
| export const useTransactionHistory = () => { |
| const [paused, setPaused] = useState(false) | ||
|
|
||
| const { data, loading, error } = useMultiChainTransactionList() | ||
| const { address } = useAccount() |
…e into new-tx-history-hooks
|
|
||
| if (withdrawal) { | ||
| return transformWithdrawal(withdrawal) | ||
| } |
There was a problem hiding this comment.
hm what's the default case here? why can it be undefined? tbh I'd rather we throw if this is unexpected
There was a problem hiding this comment.
now it will throw an error if the tx is undefined
spsjvc
changed the title
Closes #1212
Closes #1210
NewTransactionHistoryuseCompleteMultiChainTransactionsthat control the whole logicIn the next PR, the new logic will be integrated with the existing tx history